Fixes #235 : concurrency bug in FSTStreamDecoder/FSTStreamEncoder#254
Fixes #235 : concurrency bug in FSTStreamDecoder/FSTStreamEncoder#254andrewl102 wants to merge 1 commit intoRuedigerMoeller:masterfrom
Conversation
|
well, this has a major performance draw back, will try using Atomic locks or so |
|
@RuedigerMoeller Any idea of when you anticipate to have time to look into this further? |
|
will have to do performance tests before merging. fst is not thread save by intent, its just too costly in high performance use cases. Even introducing some allocations on hotspot has measurable impact. |
|
@RuedigerMoeller is this still an issue? It looks like in the docs FSTConfiguration is meant to be thread safe. Is that not the case? |
|
@perlun Thanks. Yeah it looks like a really cool project, but it's a tough problem :) We ended up just hand rolling the serialization stuff for now because I couldn't find anything that satisfied all our requirements. Also it's a game and we are really counting bytes... |
4 participants
The current implementation of the cache within these classes is not thread safe, as seen by #235 and the test case I have attached to that issue.
This might potentially have some negative performance implications but overall given the severity of the bug, I believe correctness might be more important.
Proper synchronization or protection against these fields is likely possible but beyond my current understand of the codebase, as the code involved for this is quite complex.